-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[improvement] Support auto-applying error-prone suggested fixes #660
Conversation
.toFile(); | ||
javaCompile.getOutputs().file(patchFile); | ||
|
||
// TODO(gatesn): How can we flag this on/off? It appears to come with a hefty perf hit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in tritium i use https://github.com/palantir/tritium/blob/develop/build.gradle#L110 and https://github.com/palantir/tritium/blob/develop/gradle.properties#L4 then when I want to generate a patch one can ./gradlew -DgenerateErrorPronePatch=true compileJava compileTestJava
errorProneOptions.getErrorproneArgumentProviders().add(() -> ImmutableList.of( | ||
"-XepPatchChecks:" + Joiner.on(',') | ||
.join(errorProneExtension.getPatchChecks().get()), | ||
"-XepPatchLocation:" + patchFile.getParentFile().getAbsolutePath())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use in place application to directly apply fixes similar to spotlessApply
"-XepPatchLocation:" + patchFile.getParentFile().getAbsolutePath())); | |
"-XepPatchLocation:IN_PLACE")); |
.configure(ErrorProneOptions.class, errorProneOptions -> { | ||
errorProneOptions.setEnabled(true); | ||
errorProneOptions.setDisableWarningsInGeneratedCode(true); | ||
errorProneOptions.check("EqualsHashCode", CheckSeverity.ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we read this from the extension?
file('src/main/java/test/Test.java') << inValidJavaFile | ||
|
||
then: | ||
BuildResult result = with('compileJava', "-P${BaselineErrorProne.PROP_ERROR_PRONE_APPLY}=true").build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of using the constant, could you hard code the value in the test? it helps prevent accidental changes to the flag
errorProneOptions.check("EqualsIncompatibleType", CheckSeverity.ERROR); | ||
errorProneOptions.check("StreamResourceLeak", CheckSeverity.ERROR); | ||
|
||
Optional.ofNullable(project.getProperties().get(PROP_ERROR_PRONE_APPLY)).ifPresent(x -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
project.hasProperty?
file('src/main/java/test/Test.java') << inValidJavaFile | ||
|
||
then: | ||
BuildResult result = with('compileJava', "-P${BaselineErrorProne.PROP_ERROR_PRONE_APPLY}=true").build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd test the actual syntax we intend to use (without =true
)
👍 |
Released 0.63.0 |
Before this PR
Error-prone compiler emits suggested fixes as log lines.
After this PR
Setting
-PerrorProneApply=true
will configureCompileJava
to apply error-prone suggested fixes to the source.